new withdrawal flow#140
Conversation
| address[] calldata legacyWithdrawalAddresses, | ||
| uint256[] calldata legacyWithdrawalAmounts, | ||
| uint256 legacyClaimedAmount | ||
| ) internal initializer { |
There was a problem hiding this comment.
it's internal, why the 'initializer'?
|
|
||
| if (targetCapacity == 0) revert InceptionOnPause(); | ||
| if (receiver == address(0)) revert InvalidAddress(); | ||
| if (targetCapacity == 0) revert NullParams(); |
There was a problem hiding this comment.
I think the error name for (targetCapacity == 0) should be different than (iShares == 0)
| whenNotPaused | ||
| nonReentrant | ||
| { | ||
| function withdraw( |
There was a problem hiding this comment.
withdraw implies that a certain amount of assets are to be withdrawn. However, the parameter here is in shares. I believe the name of this method should be changed to something like requestRedeem (Like in ERC7540 Async vault) to avoid confusing it with ERC4626 withdraw assets and synchronous 4626 redeem.
There was a problem hiding this comment.
Like we requestRedeem and then redeem after being fulfilled.
|
|
||
| /** @dev See {IERC4626-previewDeposit}. */ | ||
| function previewDeposit(uint256 assets) public view returns (uint256) { | ||
| if (assets < depositMinAmount) revert LowerMinAmount(depositMinAmount); |
There was a problem hiding this comment.
I think this should be removed since it is vault specific limit.
| function previewMint(uint256 shares) public view returns (uint256) { | ||
|
|
||
| uint256 assets = Convert.multiplyAndDivideCeil(shares, 1e18, ratio()); | ||
| if (assets < depositMinAmount) revert LowerMinAmount(depositMinAmount); |
There was a problem hiding this comment.
This should also be removed since this is vault specific limit
| * @param claimer Address to check claimable amount for | ||
| * @return Amount of claimable tokens for the specified address | ||
| */ | ||
| function claimableAmount(address claimer) public view virtual returns (uint256) { |
There was a problem hiding this comment.
Didn't fully understand purpose of this function. It is simply returning wstETH (or any asset) balance of a caller ?
| * @notice Returns the total amount available for withdrawal | ||
| * @return total Amount that can be claimed | ||
| */ | ||
| function claimableWithdrawalAmount() public view returns (uint256 total) { |
There was a problem hiding this comment.
Was single vault variant claimableWithdrawalAmount(address) removed ?
Tests/refactoring 3
Tests/refactoring: run config - phase 1
…dAmount functions are unused Remove legacy InVault_S_E2
withdrawal flow fixes after audit
5 participants
No description provided.