zlace0x
commented
1 year ago Resolution: Wont fix Reason: Checking ERC20 allowance is out of scope of EIP5827 as it is solely responsible for the additional check on renewable allowance. Also, the additional gas cost is also does not justify the additional check when it is meant to be used after allowance is delegated to the funnel.
Finally, ERC20 allowance is already checked and throw by the underlying ERC20 token.
The approvals performed in the Funnel contract are not connected with the approvals done in the _baseToken tokens.
The EIP-5827 should check if it has enough allowance in _baseToken in functions, transferFrom(), and transfer().
In situations where the allowance in _baseToken is less than the allowance calculated by Funnel, there will be data inconsistency and denial of service in transfer functions.
Path: ./src/Funnel.sol : allowance(), transferFrom(), transfer()
Recommendation: Consider checking allowance from _baseToken and compare it with _ramainingAllowance. React to the result in a friendly user manner.
Status: Reported (A fix was applied only to the allowance function. Provide a reasoning for why changes were not applied to the transferFrom and transfer functions)