diff --git a/contracts/token/ERC20/extensions/ERC7540.sol b/contracts/token/ERC20/extensions/ERC7540.sol index 9c6863d8..6e6f3cce 100644 --- a/contracts/token/ERC20/extensions/ERC7540.sol +++ b/contracts/token/ERC20/extensions/ERC7540.sol @@ -887,15 +887,24 @@ abstract contract ERC7540 is ERC165, ERC20, IERC4626, IERC7540 { */ function _consumeClaimableRedeem(uint256 /*shares*/, address /*controller*/) internal virtual returns (uint256); - /// @dev Returns the maximum assets that can be claimed via {deposit} for an async `owner`. + /** + * @dev Returns the maximum assets that can be claimed via {deposit} for an async `owner`. + * + * NOTE: This hook MUST reflect the strategy's internal claimable totals. It is also read by + * {_consumeClaimableDeposit} / {_consumeClaimableMint} to compute the locked exchange rate of a claim, + * so any deviation from internal state (e.g., per-tx caps, pauses, allowlists) will desynchronize + * pro-rata accounting and can permanently strand or brick claims. To enforce caps, pauses, or other + * policy, override the public {maxDeposit} / {maxMint} instead, those are read only by the + * entrypoint bound checks and do not affect rate math. + */ function _asyncMaxDeposit(address /*owner*/) internal view virtual returns (uint256); - /// @dev Returns the maximum shares that can be claimed via {mint} for an async `owner`. + /// @dev Returns the maximum shares that can be claimed via {mint} for an async `owner`. See {_asyncMaxDeposit}. function _asyncMaxMint(address /*owner*/) internal view virtual returns (uint256); - /// @dev Returns the maximum assets that can be claimed via {withdraw} for an async `owner`. + /// @dev Returns the maximum assets that can be claimed via {withdraw} for an async `owner`. See {_asyncMaxDeposit}. function _asyncMaxWithdraw(address /*owner*/) internal view virtual returns (uint256); - /// @dev Returns the maximum shares that can be claimed via {redeem} for an async `owner`. + /// @dev Returns the maximum shares that can be claimed via {redeem} for an async `owner`. See {_asyncMaxDeposit}. function _asyncMaxRedeem(address /*owner*/) internal view virtual returns (uint256); } diff --git a/contracts/token/ERC20/extensions/ERC7540AdminDeposit.sol b/contracts/token/ERC20/extensions/ERC7540AdminDeposit.sol index 5dfcf25a..c1c6ac04 100644 --- a/contracts/token/ERC20/extensions/ERC7540AdminDeposit.sol +++ b/contracts/token/ERC20/extensions/ERC7540AdminDeposit.sol @@ -95,11 +95,12 @@ abstract contract ERC7540AdminDeposit is ERC7540 { /// @dev Consumes `assets` from the claimable deposit and returns the proportional shares (rounded down). function _consumeClaimableDeposit(uint256 assets, address controller) internal virtual override returns (uint256) { - // When `assets` equals the controller's full claimable balance (including the case where both - // sides are 0), the entire remaining `claimableShares` is returned and consumed. This drains any - // residue left after a partial claim was rounded against the share side. - uint256 maxAssets = maxDeposit(controller); - uint256 maxShares = maxMint(controller); + // Pro-rata is computed against {_asyncMaxDeposit} / {_asyncMaxMint} (strategy state) rather than the + // public {maxDeposit} / {maxMint}, so cap/pause overrides on the public surface cannot desynchronize + // the locked rate. When `assets` equals the controller's full claimable balance (including the case + // where both sides are 0), the entire remaining `claimableShares` is returned and consumed. + uint256 maxAssets = _asyncMaxDeposit(controller); + uint256 maxShares = _asyncMaxMint(controller); uint256 shares = assets == maxAssets ? maxShares : Math.mulDiv(assets, maxShares, maxAssets, Math.Rounding.Floor); @@ -111,11 +112,12 @@ abstract contract ERC7540AdminDeposit is ERC7540 { /// @dev Consumes `shares` from the claimable deposit and returns the proportional assets (rounded up). function _consumeClaimableMint(uint256 shares, address controller) internal virtual override returns (uint256) { - // When `shares` equals the controller's full claimable balance (including the case where both - // sides are 0), the entire remaining `claimableAssets` is returned and consumed. This drains any - // residue left after a partial claim was rounded against the asset side. - uint256 maxAssets = maxDeposit(controller); - uint256 maxShares = maxMint(controller); + // Pro-rata is computed against {_asyncMaxDeposit} / {_asyncMaxMint} (strategy state) rather than the + // public {maxDeposit} / {maxMint}, so cap/pause overrides on the public surface cannot desynchronize + // the locked rate. When `shares` equals the controller's full claimable balance (including the case + // where both sides are 0), the entire remaining `claimableAssets` is returned and consumed. + uint256 maxAssets = _asyncMaxDeposit(controller); + uint256 maxShares = _asyncMaxMint(controller); uint256 assets = shares == maxShares ? maxAssets : Math.mulDiv(shares, maxAssets, maxShares, Math.Rounding.Ceil); diff --git a/contracts/token/ERC20/extensions/ERC7540AdminRedeem.sol b/contracts/token/ERC20/extensions/ERC7540AdminRedeem.sol index 5a4f6da1..d744bee5 100644 --- a/contracts/token/ERC20/extensions/ERC7540AdminRedeem.sol +++ b/contracts/token/ERC20/extensions/ERC7540AdminRedeem.sol @@ -90,11 +90,12 @@ abstract contract ERC7540AdminRedeem is ERC7540 { /// @dev Consumes `assets` from the claimable redeem and returns the proportional shares (rounded up). function _consumeClaimableWithdraw(uint256 assets, address controller) internal virtual override returns (uint256) { - // When `assets` equals the controller's full claimable balance (including the case where both - // sides are 0), the entire remaining `claimableShares` is returned and consumed. This drains any - // residue left after a partial claim was rounded against the share side. - uint256 maxAssets = maxWithdraw(controller); - uint256 maxShares = maxRedeem(controller); + // Pro-rata is computed against {_asyncMaxWithdraw} / {_asyncMaxRedeem} (strategy state) rather than the + // public {maxWithdraw} / {maxRedeem}, so cap/pause overrides on the public surface cannot desynchronize + // the locked rate. When `assets` equals the controller's full claimable balance (including the case + // where both sides are 0), the entire remaining `claimableShares` is returned and consumed. + uint256 maxAssets = _asyncMaxWithdraw(controller); + uint256 maxShares = _asyncMaxRedeem(controller); uint256 shares = assets == maxAssets ? maxShares : Math.mulDiv(assets, maxShares, maxAssets, Math.Rounding.Ceil); @@ -106,11 +107,12 @@ abstract contract ERC7540AdminRedeem is ERC7540 { /// @dev Consumes `shares` from the claimable redeem and returns the proportional assets (rounded down). function _consumeClaimableRedeem(uint256 shares, address controller) internal virtual override returns (uint256) { - // When `shares` equals the controller's full claimable balance (including the case where both - // sides are 0), the entire remaining `claimableAssets` is returned and consumed. This drains any - // residue left after a partial claim was rounded against the asset side. - uint256 maxShares = maxRedeem(controller); - uint256 maxAssets = maxWithdraw(controller); + // Pro-rata is computed against {_asyncMaxWithdraw} / {_asyncMaxRedeem} (strategy state) rather than the + // public {maxWithdraw} / {maxRedeem}, so cap/pause overrides on the public surface cannot desynchronize + // the locked rate. When `shares` equals the controller's full claimable balance (including the case + // where both sides are 0), the entire remaining `claimableAssets` is returned and consumed. + uint256 maxShares = _asyncMaxRedeem(controller); + uint256 maxAssets = _asyncMaxWithdraw(controller); uint256 assets = shares == maxShares ? maxAssets : Math.mulDiv(shares, maxAssets, maxShares, Math.Rounding.Floor); diff --git a/contracts/token/ERC20/extensions/ERC7540DelayDeposit.sol b/contracts/token/ERC20/extensions/ERC7540DelayDeposit.sol index f5e1ba7a..b5a5f965 100644 --- a/contracts/token/ERC20/extensions/ERC7540DelayDeposit.sol +++ b/contracts/token/ERC20/extensions/ERC7540DelayDeposit.sol @@ -95,22 +95,32 @@ abstract contract ERC7540DelayDeposit is ERC7540, IERC6372 { return super._requestDeposit(assets, controller, owner, timepoint); } - /** - * @dev Consumes `assets` from claimable deposits, returns proportional shares (rounded down). - * - * Requirements: - * - * * {maxMint} must not be 0 for `controller`. Panics with division by zero otherwise. - */ + /// @dev Consumes `assets` from claimable deposits, returns proportional shares (rounded down). function _consumeClaimableDeposit(uint256 assets, address controller) internal virtual override returns (uint256) { - uint256 shares = Math.mulDiv(assets, maxMint(controller), maxDeposit(controller), Math.Rounding.Floor); + // Pro-rata is computed against {_asyncMaxDeposit} / {_asyncMaxMint} (strategy state) rather than the + // public {maxDeposit} / {maxMint}, so cap/pause overrides on the public surface cannot desynchronize + // the rate used for consumption. + uint256 shares = Math.mulDiv( + assets, + _asyncMaxMint(controller), + _asyncMaxDeposit(controller), + Math.Rounding.Floor + ); _claimedDeposits[controller] += assets; return shares; } /// @dev Consumes `shares` from claimable deposits, returns proportional assets (rounded up). function _consumeClaimableMint(uint256 shares, address controller) internal virtual override returns (uint256) { - uint256 assets = Math.mulDiv(shares, maxDeposit(controller), maxMint(controller), Math.Rounding.Ceil); + // Pro-rata is computed against {_asyncMaxDeposit} / {_asyncMaxMint} (strategy state) rather than the + // public {maxDeposit} / {maxMint}, so cap/pause overrides on the public surface cannot desynchronize + // the rate used for consumption. + uint256 assets = Math.mulDiv( + shares, + _asyncMaxDeposit(controller), + _asyncMaxMint(controller), + Math.Rounding.Ceil + ); _claimedDeposits[controller] += assets; return assets; } diff --git a/contracts/token/ERC20/extensions/ERC7540DelayRedeem.sol b/contracts/token/ERC20/extensions/ERC7540DelayRedeem.sol index 4972284f..c1d32c70 100644 --- a/contracts/token/ERC20/extensions/ERC7540DelayRedeem.sol +++ b/contracts/token/ERC20/extensions/ERC7540DelayRedeem.sol @@ -97,14 +97,30 @@ abstract contract ERC7540DelayRedeem is ERC7540, IERC6372 { /// @dev Consumes `assets` from claimable redeems, returns proportional shares (rounded up). function _consumeClaimableWithdraw(uint256 assets, address controller) internal virtual override returns (uint256) { - uint256 shares = Math.mulDiv(assets, maxRedeem(controller), maxWithdraw(controller), Math.Rounding.Ceil); + // Pro-rata is computed against {_asyncMaxWithdraw} / {_asyncMaxRedeem} (strategy state) rather than the + // public {maxWithdraw} / {maxRedeem}, so cap/pause overrides on the public surface cannot desynchronize + // the rate used for consumption. + uint256 shares = Math.mulDiv( + assets, + _asyncMaxRedeem(controller), + _asyncMaxWithdraw(controller), + Math.Rounding.Ceil + ); _claimedRedeems[controller] += shares; return shares; } /// @dev Consumes `shares` from claimable redeems, returns proportional assets (rounded down). function _consumeClaimableRedeem(uint256 shares, address controller) internal virtual override returns (uint256) { - uint256 assets = Math.mulDiv(shares, maxWithdraw(controller), maxRedeem(controller), Math.Rounding.Floor); + // Pro-rata is computed against {_asyncMaxWithdraw} / {_asyncMaxRedeem} (strategy state) rather than the + // public {maxWithdraw} / {maxRedeem}, so cap/pause overrides on the public surface cannot desynchronize + // the rate used for consumption. + uint256 assets = Math.mulDiv( + shares, + _asyncMaxWithdraw(controller), + _asyncMaxRedeem(controller), + Math.Rounding.Floor + ); _claimedRedeems[controller] += shares; return assets; }