The `_zapIn` function may unexpectedly revert due to the incorrect implementation of `_transferTokenInAndApprove`
The `_transferTokenInAndApprove` function should approve the `router` on behalf of the VaultZapper contract. However, it checks the allowance from `msgSender` to the `router`, rather than the VaultZapper. This potentially results in the VaultZapper not approving the `router` and causing unexpected reverting.\\nThe allowance check in the `_transferTokenInAndApprove` function should verify that `address(this)` has approved sufficient amount of `tokenIn` to the `router`. However, it currently checks the allowance of `_msgSender()`, which is unnecessary and may cause transaction reverting if `_msgSender` had previously approved the `router`.\\n```\\n    function _transferTokenInAndApprove(\\n        address router,\\n        IERC20 tokenIn,\\n        uint256 amount\\n    )\\n        internal\\n    {\\n        tokenIn.safeTransferFrom(_msgSender(), address(this), amount);\\n//@ The check of allowance is useless, we should check the allowance from address(this) rather than the msgSender\\n        if (tokenIn.allowance(_msgSender(), router) < amount) {\\n            tokenIn.forceApprove(router, amount);\\n        }\\n    }\\n```\\n\\nPOC\\nApply the patch to `asynchronous-vault/test/Zapper/ZapperDeposit.t.sol` to add the test case and run it with `forge test --match-test test_zapIn --ffi`.\\n```\\ndiff --git a/asynchronous-vault/test/Zapper/ZapperDeposit.t.sol b/asynchronous-vault/test/Zapper/ZapperDeposit.t.sol\\nindex 9083127..ff11b56 100644\\n--- a/asynchronous-vault/test/Zapper/ZapperDeposit.t.sol\\n// Add the line below\\n// Add the line below\\n// Add the line below\\n b/asynchronous-vault/test/Zapper/ZapperDeposit.t.sol\\n@@ -17,6 // Add the line below\\n17,25 @@ contract VaultZapperDeposit is OffChainCalls {\\n         zapper = new VaultZapper();\\n     }\\n\\n// Add the line below\\n    function test_zapIn() public {\\n// Add the line below\\n        Swap memory params =\\n// Add the line below\\n            Swap(_router, _USDC, _WSTETH, 1500 * 1e6, 1, address(0), 20);\\n// Add the line below\\n        _setUpVaultAndZapper(_WSTETH);\\n// Add the line below\\n\\n// Add the line below\\n        IERC4626 vault = _vault;\\n// Add the line below\\n        bytes memory swapData =\\n// Add the line below\\n            _getSwapData(address(zapper), address(zapper), params);\\n// Add the line below\\n\\n// Add the line below\\n        _getTokenIn(params);\\n// Add the line below\\n\\n// Add the line below\\n        // If the msgSender() happend to approve the SwapRouter before, then the zap will always revert\\n// Add the line below\\n        IERC20(params.tokenIn).approve(address(params.router), params.amount);\\n// Add the line below\\n        zapper.zapAndDeposit(\\n// Add the line below\\n            params.tokenIn, vault, params.router, params.amount, swapData\\n// Add the line below\\n        );\\n// Add the line below\\n\\n// Add the line below\\n    }\\n// Add the line below\\n\\n     //// test_zapAndDeposit ////\\n     function test_zapAndDepositUsdcWSTETH() public {\\n         Swap memory usdcToWstEth =\\n```\\n\\nResult:\\n```\\nRan 1 test for test/Zapper/ZapperDeposit.t.sol:VaultZapperDeposit\\n[FAIL. Reason: SwapFailed("\\u{8}�y�\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0 \\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0(ERC20: transfer amount exceeds allowance\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0")] test_zapIn() (gas: 4948462)\\nSuite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 20.84s (18.74s CPU time)\\n\\nRan 1 test suite in 22.40s (20.84s CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)\\n\\nFailing tests:\\nEncountered 1 failing test in test/Zapper/ZapperDeposit.t.sol:VaultZapperDeposit\\n[FAIL. Reason: SwapFailed("\\u{8}�y�\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0 \\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0(ERC20: transfer amount exceeds allowance\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0\\0")] test_zapIn() (gas: 4948462)\\n```\\n
Fix the issue:\\n```\\ndiff // Remove the line below\\n// Remove the line below\\ngit a/asynchronous// Remove the line below\\nvault/src/VaultZapper.sol b/asynchronous// Remove the line below\\nvault/src/VaultZapper.sol\\nindex 9943535..9cf6df9 100644\\n// Remove the line below\\n// Remove the line below\\n// Remove the line below\\n a/asynchronous// Remove the line below\\nvault/src/VaultZapper.sol\\n// Add the line below\\n// Add the line below\\n// Add the line below\\n b/asynchronous// Remove the line below\\nvault/src/VaultZapper.sol\\n@@ // Remove the line below\\n165,7 // Add the line below\\n165,7 @@ contract VaultZapper is Ownable2Step, Pausable {\\n         internal\\n     {\\n         tokenIn.safeTransferFrom(_msgSender(), address(this), amount);\\n// Remove the line below\\n        if (tokenIn.allowance(_msgSender(), router) < amount) {\\n// Add the line below\\n        if (tokenIn.allowance(address(this), router) < amount) {\\n             tokenIn.forceApprove(router, amount);\\n         }\\n     }\\n```\\n
This issue could lead to transaction reverting when users interact with the contract normally, thereby affecting the contract's regular functionality.
```\\n    function _transferTokenInAndApprove(\\n        address router,\\n        IERC20 tokenIn,\\n        uint256 amount\\n    )\\n        internal\\n    {\\n        tokenIn.safeTransferFrom(_msgSender(), address(this), amount);\\n//@ The check of allowance is useless, we should check the allowance from address(this) rather than the msgSender\\n        if (tokenIn.allowance(_msgSender(), router) < amount) {\\n            tokenIn.forceApprove(router, amount);\\n        }\\n    }\\n```\\n