Add a "fees" section to the ERC4626 guide (#4054)

Co-authored-by: Francisco Giordano <fg@frang.io>

contracts/mocks/docs/ERC4626Fees.sol

@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import "../../token/ERC20/extensions/ERC4626.sol";
+
+abstract contract ERC4626Fees is ERC4626 {
+    using Math for uint256;
+
+    /** @dev See {IERC4626-previewDeposit}. */
+    function previewDeposit(uint256 assets) public view virtual override returns (uint256) {
+        uint256 fee = _feeOnTotal(assets, _entryFeeBasePoint());
+        return super.previewDeposit(assets - fee);
+    }
+
+    /** @dev See {IERC4626-previewMint}. */
+    function previewMint(uint256 shares) public view virtual override returns (uint256) {
+        uint256 assets = super.previewMint(shares);
+        return assets + _feeOnRaw(assets, _entryFeeBasePoint());
+    }
+
+    /** @dev See {IERC4626-previewWithdraw}. */
+    function previewWithdraw(uint256 assets) public view virtual override returns (uint256) {
+        uint256 fee = _feeOnRaw(assets, _exitFeeBasePoint());
+        return super.previewWithdraw(assets + fee);
+    }
+
+    /** @dev See {IERC4626-previewRedeem}. */
+    function previewRedeem(uint256 shares) public view virtual override returns (uint256) {
+        uint256 assets = super.previewRedeem(shares);
+        return assets - _feeOnTotal(assets, _exitFeeBasePoint());
+    }
+
+    /** @dev See {IERC4626-_deposit}. */
+    function _deposit(address caller, address receiver, uint256 assets, uint256 shares) internal virtual override {
+        uint256 fee = _feeOnTotal(assets, _entryFeeBasePoint());
+        address recipient = _entryFeeRecipient();
+
+        super._deposit(caller, receiver, assets, shares);
+
+        if (fee > 0 && recipient != address(this)) {
+            SafeERC20.safeTransfer(IERC20(asset()), recipient, fee);
+        }
+    }
+
+    /** @dev See {IERC4626-_deposit}. */
+    function _withdraw(
+        address caller,
+        address receiver,
+        address owner,
+        uint256 assets,
+        uint256 shares
+    ) internal virtual override {
+        uint256 fee = _feeOnRaw(assets, _exitFeeBasePoint());
+        address recipient = _exitFeeRecipient();
+
+        super._withdraw(caller, receiver, owner, assets, shares);
+
+        if (fee > 0 && recipient != address(this)) {
+            SafeERC20.safeTransfer(IERC20(asset()), recipient, fee);
+        }
+    }
+
+    function _entryFeeBasePoint() internal view virtual returns (uint256) {
+        return 0;
+    }
+
+    function _entryFeeRecipient() internal view virtual returns (address) {
+        return address(0);
+    }
+
+    function _exitFeeBasePoint() internal view virtual returns (uint256) {
+        return 0;
+    }
+
+    function _exitFeeRecipient() internal view virtual returns (address) {
+        return address(0);
+    }
+
+    function _feeOnRaw(uint256 assets, uint256 feeBasePoint) private pure returns (uint256) {
+        return assets.mulDiv(feeBasePoint, 1e5, Math.Rounding.Up);
+    }
+
+    function _feeOnTotal(uint256 assets, uint256 feeBasePoint) private pure returns (uint256) {
+        return assets.mulDiv(feeBasePoint, feeBasePoint + 1e5, Math.Rounding.Up);
+    }
+}

contracts/mocks/token/ERC4646FeesMock.sol

@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import "../docs/ERC4626Fees.sol";
+
+abstract contract ERC4626FeesMock is ERC4626Fees {
+    uint256 private immutable _entryFeeBasePointValue;
+    address private immutable _entryFeeRecipientValue;
+    uint256 private immutable _exitFeeBasePointValue;
+    address private immutable _exitFeeRecipientValue;
+
+    constructor(
+        uint256 entryFeeBasePoint,
+        address entryFeeRecipient,
+        uint256 exitFeeBasePoint,
+        address exitFeeRecipient
+    ) {
+        _entryFeeBasePointValue = entryFeeBasePoint;
+        _entryFeeRecipientValue = entryFeeRecipient;
+        _exitFeeBasePointValue = exitFeeBasePoint;
+        _exitFeeRecipientValue = exitFeeRecipient;
+    }
+
+    function _entryFeeBasePoint() internal view virtual override returns (uint256) {
+        return _entryFeeBasePointValue;
+    }
+
+    function _entryFeeRecipient() internal view virtual override returns (address) {
+        return _entryFeeRecipientValue;
+    }
+
+    function _exitFeeBasePoint() internal view virtual override returns (uint256) {
+        return _exitFeeBasePointValue;
+    }
+
+    function _exitFeeRecipient() internal view virtual override returns (address) {
+        return _exitFeeRecipientValue;
+    }
+}

docs/modules/ROOT/pages/erc4626.adoc

@@ -191,3 +191,25 @@ stem:[\delta = 3], stem:[a_0 = 100], stem:[a_1 = 10^5]
 
 image::erc4626-attack-6.png[Inflation attack without offset=6]
 stem:[\delta = 6], stem:[a_0 = 1], stem:[a_1 = 10^5]
+
+
+[[fees]]
+== Custom behavior: Adding fees to the vault
+
+In an ERC4626 vaults, fees can be captured during the deposit/mint and/or during the withdraw/redeem steps. In both cases it is essential to remain compliant with the ERC4626 requirements with regard to the preview functions.
+
+For example, if calling `deposit(100, receiver)`, the caller should deposit exactly 100 underlying tokens, including fees, and the receiver should receive a number of shares that matches the value returned by `previewDeposit(100)`. Similarly, `previewMint` should account for the fees that the user will have to pay on top of share's cost.
+
+As for the `Deposit` event, while this is less clear in the EIP spec itself, there seems to be consensus that it should include the number of assets paid for by the user, including the fees.
+
+On the other hand, when withdrawing assets, the number given by the user should correspond to what he receives. Any fees should be added to the quote (in shares) performed by `previewWithdraw`.
+
+The `Withdraw` event should include the number of shares the user burns (including fees) and the number of assets the user actually receives (after fees are deducted).
+
+The consequence of this design is that both the `Deposit` and `Withdraw` events will describe two exchange rates. The spread between the "Buy-in" and the "Exit" prices correspond to the fees taken by the vault.
+
+The following example describes how fees proportional to the deposited/withdrawn amount can be implemented:
+
+```solidity
+include::api:example$ERC4626Fees.sol[]
+```

scripts/prepare-docs.sh

@@ -12,4 +12,12 @@ rm -rf "$OUTDIR"
 
 hardhat docgen
 
+# copy examples and adjust imports
+examples_dir="docs/modules/api/examples"
+mkdir -p "$examples_dir"
+for f in contracts/mocks/docs/*.sol; do
+  name="$(basename "$f")"
+  sed -e '/^import/s|\.\./\.\./|@openzeppelin/contracts/|' "$f" > "docs/modules/api/examples/$name"
+done
+
 node scripts/gen-nav.js "$OUTDIR" > "$OUTDIR/../nav.adoc"

test/token/ERC20/extensions/ERC4626.test.js

@@ -4,6 +4,7 @@ const { expect } = require('chai');
 const ERC20Decimals = artifacts.require('$ERC20DecimalsMock');
 const ERC4626 = artifacts.require('$ERC4626');
 const ERC4626OffsetMock = artifacts.require('$ERC4626OffsetMock');
+const ERC4626FeesMock = artifacts.require('$ERC4626FeesMock');
 
 contract('ERC4626', function (accounts) {
   const [holder, recipient, spender, other, user1, user2] = accounts;
@@ -489,6 +490,132 @@ contract('ERC4626', function (accounts) {
     });
   }
 
+  describe('ERC4626Fees', function () {
+    const feeBasePoint = web3.utils.toBN(5e3);
+    const amountWithoutFees = web3.utils.toBN(10000);
+    const fees = amountWithoutFees.mul(feeBasePoint).divn(1e5);
+    const amountWithFees = amountWithoutFees.add(fees);
+
+    describe('input fees', function () {
+      beforeEach(async function () {
+        this.token = await ERC20Decimals.new(name, symbol, 18);
+        this.vault = await ERC4626FeesMock.new(
+          name + ' Vault',
+          symbol + 'V',
+          this.token.address,
+          feeBasePoint,
+          other,
+          0,
+          constants.ZERO_ADDRESS,
+        );
+
+        await this.token.$_mint(holder, constants.MAX_INT256);
+        await this.token.approve(this.vault.address, constants.MAX_INT256, { from: holder });
+      });
+
+      it('deposit', async function () {
+        expect(await this.vault.previewDeposit(amountWithFees)).to.be.bignumber.equal(amountWithoutFees);
+        ({ tx: this.tx } = await this.vault.deposit(amountWithFees, recipient, { from: holder }));
+      });
+
+      it('mint', async function () {
+        expect(await this.vault.previewMint(amountWithoutFees)).to.be.bignumber.equal(amountWithFees);
+        ({ tx: this.tx } = await this.vault.mint(amountWithoutFees, recipient, { from: holder }));
+      });
+
+      afterEach(async function () {
+        // get total
+        await expectEvent.inTransaction(this.tx, this.token, 'Transfer', {
+          from: holder,
+          to: this.vault.address,
+          value: amountWithFees,
+        });
+
+        // redirect fees
+        await expectEvent.inTransaction(this.tx, this.token, 'Transfer', {
+          from: this.vault.address,
+          to: other,
+          value: fees,
+        });
+
+        // mint shares
+        await expectEvent.inTransaction(this.tx, this.vault, 'Transfer', {
+          from: constants.ZERO_ADDRESS,
+          to: recipient,
+          value: amountWithoutFees,
+        });
+
+        // deposit event
+        await expectEvent.inTransaction(this.tx, this.vault, 'Deposit', {
+          sender: holder,
+          owner: recipient,
+          assets: amountWithFees,
+          shares: amountWithoutFees,
+        });
+      });
+    });
+
+    describe('output fees', function () {
+      beforeEach(async function () {
+        this.token = await ERC20Decimals.new(name, symbol, 18);
+        this.vault = await ERC4626FeesMock.new(
+          name + ' Vault',
+          symbol + 'V',
+          this.token.address,
+          0,
+          constants.ZERO_ADDRESS,
+          5e3, // 5%
+          other,
+        );
+
+        await this.token.$_mint(this.vault.address, constants.MAX_INT256);
+        await this.vault.$_mint(holder, constants.MAX_INT256);
+      });
+
+      it('redeem', async function () {
+        expect(await this.vault.previewRedeem(amountWithFees)).to.be.bignumber.equal(amountWithoutFees);
+        ({ tx: this.tx } = await this.vault.redeem(amountWithFees, recipient, holder, { from: holder }));
+      });
+
+      it('withdraw', async function () {
+        expect(await this.vault.previewWithdraw(amountWithoutFees)).to.be.bignumber.equal(amountWithFees);
+        ({ tx: this.tx } = await this.vault.withdraw(amountWithoutFees, recipient, holder, { from: holder }));
+      });
+
+      afterEach(async function () {
+        // withdraw principal
+        await expectEvent.inTransaction(this.tx, this.token, 'Transfer', {
+          from: this.vault.address,
+          to: recipient,
+          value: amountWithoutFees,
+        });
+
+        // redirect fees
+        await expectEvent.inTransaction(this.tx, this.token, 'Transfer', {
+          from: this.vault.address,
+          to: other,
+          value: fees,
+        });
+
+        // mint shares
+        await expectEvent.inTransaction(this.tx, this.vault, 'Transfer', {
+          from: holder,
+          to: constants.ZERO_ADDRESS,
+          value: amountWithFees,
+        });
+
+        // withdraw event
+        await expectEvent.inTransaction(this.tx, this.vault, 'Withdraw', {
+          sender: holder,
+          receiver: recipient,
+          owner: holder,
+          assets: amountWithoutFees,
+          shares: amountWithFees,
+        });
+      });
+    });
+  });
+
   /// Scenario inspired by solmate ERC4626 tests:
   /// https://github.com/transmissions11/solmate/blob/main/src/test/ERC4626.t.sol
   it('multiple mint, deposit, redeem & withdrawal', async function () {