diff --git a/.changeset/shiny-dolphins-lick.md b/.changeset/shiny-dolphins-lick.md new file mode 100644 index 00000000000..508e7f4304c --- /dev/null +++ b/.changeset/shiny-dolphins-lick.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`ERC4626`: compute `maxWithdraw` using `maxRedeem` and `previewRedeem` so that changes to the preview functions affect the max functions. diff --git a/contracts/interfaces/IERC4626.sol b/contracts/interfaces/IERC4626.sol index 9a24507a7fd..dbd9bf403aa 100644 --- a/contracts/interfaces/IERC4626.sol +++ b/contracts/interfaces/IERC4626.sol @@ -198,7 +198,7 @@ interface IERC4626 is IERC20, IERC20Metadata { function maxRedeem(address owner) external view returns (uint256 maxShares); /** - * @dev Allows an on-chain or off-chain user to simulate the effects of their redeemption at the current block, + * @dev Allows an on-chain or off-chain user to simulate the effects of their redemption at the current block, * given current on-chain conditions. * * - MUST return as close to and no more than the exact amount of assets that would be withdrawn in a redeem call diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index c71b14ad48c..7e77c9d4440 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -44,6 +44,26 @@ import {Math} from "../../../utils/math/Math.sol"; * * To learn more, check out our xref:ROOT:erc4626.adoc[ERC-4626 guide]. * ==== + * + * [NOTE] + * ==== + * When overriding this contract, some elements must to be considered: + * + * * When overriding the behavior of the deposit or withdraw mechanisms, it is recommended to override the internal + * functions. Overriding {_deposit} automatically affects both {deposit} and {mint}. Similarly, overriding {_withdraw} + * automatically affects both {withdraw} and {redeem}. Overall it is not recommended to override the public facing + * functions since that could lead to inconsistent behaviors between the {deposit} and {mint} or between {withdraw} and + * {redeem}, which is documented to have lead to loss of funds. + * + * * Overrides to the deposit or withdraw mechanism must be reflected in the preview functions as well. + * + * * {maxWithdraw} depends on {maxRedeem}. Therefore, overriding {maxRedeem} only is enough. On the other hand, + * overriding {maxWithdraw} only would have no effect on {maxRedeem}, and could create an inconsistency between the two + * functions. + * + * * If {previewRedeem} is overridden to revert, {maxWithdraw} must be overridden as necessary to ensure it + * always return successfully. + * ==== */ abstract contract ERC4626 is ERC20, IERC4626 { using Math for uint256; @@ -107,67 +127,67 @@ abstract contract ERC4626 is ERC20, IERC4626 { return _underlyingDecimals + _decimalsOffset(); } - /** @dev See {IERC4626-asset}. */ + /// @inheritdoc IERC4626 function asset() public view virtual returns (address) { return address(_asset); } - /** @dev See {IERC4626-totalAssets}. */ + /// @inheritdoc IERC4626 function totalAssets() public view virtual returns (uint256) { return _asset.balanceOf(address(this)); } - /** @dev See {IERC4626-convertToShares}. */ + /// @inheritdoc IERC4626 function convertToShares(uint256 assets) public view virtual returns (uint256) { return _convertToShares(assets, Math.Rounding.Floor); } - /** @dev See {IERC4626-convertToAssets}. */ + /// @inheritdoc IERC4626 function convertToAssets(uint256 shares) public view virtual returns (uint256) { return _convertToAssets(shares, Math.Rounding.Floor); } - /** @dev See {IERC4626-maxDeposit}. */ + /// @inheritdoc IERC4626 function maxDeposit(address) public view virtual returns (uint256) { return type(uint256).max; } - /** @dev See {IERC4626-maxMint}. */ + /// @inheritdoc IERC4626 function maxMint(address) public view virtual returns (uint256) { return type(uint256).max; } - /** @dev See {IERC4626-maxWithdraw}. */ + /// @inheritdoc IERC4626 function maxWithdraw(address owner) public view virtual returns (uint256) { - return _convertToAssets(balanceOf(owner), Math.Rounding.Floor); + return previewRedeem(maxRedeem(owner)); } - /** @dev See {IERC4626-maxRedeem}. */ + /// @inheritdoc IERC4626 function maxRedeem(address owner) public view virtual returns (uint256) { return balanceOf(owner); } - /** @dev See {IERC4626-previewDeposit}. */ + /// @inheritdoc IERC4626 function previewDeposit(uint256 assets) public view virtual returns (uint256) { return _convertToShares(assets, Math.Rounding.Floor); } - /** @dev See {IERC4626-previewMint}. */ + /// @inheritdoc IERC4626 function previewMint(uint256 shares) public view virtual returns (uint256) { return _convertToAssets(shares, Math.Rounding.Ceil); } - /** @dev See {IERC4626-previewWithdraw}. */ + /// @inheritdoc IERC4626 function previewWithdraw(uint256 assets) public view virtual returns (uint256) { return _convertToShares(assets, Math.Rounding.Ceil); } - /** @dev See {IERC4626-previewRedeem}. */ + /// @inheritdoc IERC4626 function previewRedeem(uint256 shares) public view virtual returns (uint256) { return _convertToAssets(shares, Math.Rounding.Floor); } - /** @dev See {IERC4626-deposit}. */ + /// @inheritdoc IERC4626 function deposit(uint256 assets, address receiver) public virtual returns (uint256) { uint256 maxAssets = maxDeposit(receiver); if (assets > maxAssets) { @@ -180,7 +200,7 @@ abstract contract ERC4626 is ERC20, IERC4626 { return shares; } - /** @dev See {IERC4626-mint}. */ + /// @inheritdoc IERC4626 function mint(uint256 shares, address receiver) public virtual returns (uint256) { uint256 maxShares = maxMint(receiver); if (shares > maxShares) { @@ -193,7 +213,7 @@ abstract contract ERC4626 is ERC20, IERC4626 { return assets; } - /** @dev See {IERC4626-withdraw}. */ + /// @inheritdoc IERC4626 function withdraw(uint256 assets, address receiver, address owner) public virtual returns (uint256) { uint256 maxAssets = maxWithdraw(owner); if (assets > maxAssets) { @@ -206,7 +226,7 @@ abstract contract ERC4626 is ERC20, IERC4626 { return shares; } - /** @dev See {IERC4626-redeem}. */ + /// @inheritdoc IERC4626 function redeem(uint256 shares, address receiver, address owner) public virtual returns (uint256) { uint256 maxShares = maxRedeem(owner); if (shares > maxShares) {