Submitted by unforgiven, also found by adriro
https://github.com/AstariaXYZ/astaria-gpl/blob/4b49fe993d9b807fe68b3421ee7f2fe91267c9ef/src/ERC4626RouterBase.sol#L48
https://github.com/AstariaXYZ/astaria-gpl/blob/4b49fe993d9b807fe68b3421ee7f2fe91267c9ef/src/ERC4626RouterBase.sol#L62
Functions withdraw() and redeem()  in ERC4626RouterBase  are used to withdraw user funds from vaults and they call vault.withdraw() and vault.redeem() and logics in vault transfer user shares and user required to give spending allowance for vault and there is no need for ERC4626RouterBase to set approval for vault and because those approved tokens wont be used and code uses safeApprove() so next calls to withdraw() and redeem() would revert because code would tries to change allowance amount while its not zero. those functions would revert always and AstariaRouter uses them and user wont be able to use those function and any other protocol integrating with Astaria calling those function would have broken logic. also if UI interact with protocol with router functions then UI would have broken parts too. and functions in router support users to set slippage allowance and without them users have to interact with vault directly and they may lose funds because of the slippage.
This is withdraw() and redeem() code in ERC4626RouterBase:
As you can see the code sets approval for vault to spend routers vault tokens and then call vault function. This is _redeemFutureEpoch() code in the vault which handles withdraw and redeem:
As you can see this code only checks spending allowance that real owner of shares gives to the msg.sender and there is no check or updating spending allowance of the router vaulttokens for vault. so those approvals in the withdraw() and redeem() are unnecessary and they would cause code to revert always because code tries to set approval with safeApprove() while the current allowance is not zero.
This issue would cause calls to withdraw() and redeem() function to revert. any other protocol integrating with Astaria using those functions would have broken logic and also users would lose gas if they use those functions. contract AstariaRouter inherits ERC4626RouterBase and uses its withdraw() and redeem() function so users cant call AstariaRouter.withdraw() or AstariaRouter.redeem()which supports slippage allowance and they have to call vaults functions directly and they may lose funds because of the slippage.
VIM
Remove unnecessary code.
androolloyd (Astaria) confirmed
