Incorrect shares accounting cause liquidations to fail in some cases
Accounting mismatch when marking claimable yield against the vault's shares may cause failing liquidations.\\n`withdraw_underlying_to_claim()` distributes `_amount_shares` worth of underlying tokens (WETH) to token holders. Note that this burns the shares held by the vault, but for accounting purposes, the `total_shares` variable isn't updated.\\nHowever, if a token holder chooses to liquidate his shares, his `shares_owned` are used entirely in both `alchemist.liquidate()` and `withdrawUnderlying()`. Because the contract no longer has fewer shares as a result of the yield distribution, the liquidation will fail.\\nPOC\\nRefer to the `testVaultLiquidationAfterRepayment()` test case below. Note that this requires a fix to be applied for #2 first.\\n```\\n// SPDX-License-Identifier: MIT\\npragma solidity 0.8.18;\\n\\nimport "forge-std/Test.sol";\\nimport "../../lib/utils/VyperDeployer.sol";\\n\\nimport "../IVault.sol";\\nimport "../IAlchemistV2.sol";\\nimport "../MintableERC721.sol";\\nimport "openzeppelin/token/ERC20/IERC20.sol";\\n\\ncontract VaultTest is Test {\\n    ///@notice create a new instance of VyperDeployer\\n    VyperDeployer vyperDeployer = new VyperDeployer();\\n\\n    FairFundingToken nft;\\n    IVault vault;\\n    address vaultAdd;\\n    IAlchemistV2 alchemist = IAlchemistV2(0x062Bf725dC4cDF947aa79Ca2aaCCD4F385b13b5c);\\n    IWhitelist whitelist = IWhitelist(0xA3dfCcbad1333DC69997Da28C961FF8B2879e653);\\n    address yieldToken = 0xa258C4606Ca8206D8aA700cE2143D7db854D168c;\\n    IERC20 weth = IERC20(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2);\\n    // pranking from big WETH holder\\n    address admin = 0x2fEb1512183545f48f6b9C5b4EbfCaF49CfCa6F3;\\n    address user1 = address(0x123);\\n    address user2 = address(0x456);\\n    \\n    function setUp() public {\\n        vm.startPrank(admin);\\n        nft = new FairFundingToken();\\n        /// @notice: I modified vault to take in admin as a parameter\\n        /// because of pranking issues => setting permissions\\n        vault = IVault(\\n            vyperDeployer.deployContract("Vault", abi.encode(address(nft), admin))\\n        );\\n        // to avoid having to repeatedly cast to address\\n        vaultAdd = address(vault);\\n        vault.set_alchemist(address(alchemist));\\n\\n        // whitelist vault and users in Alchemist system, otherwise will run into permission issues\\n        vm.stopPrank();\\n        vm.startPrank(0x9e2b6378ee8ad2A4A95Fe481d63CAba8FB0EBBF9);\\n        whitelist.add(vaultAdd);\\n        whitelist.add(admin);\\n        whitelist.add(user1);\\n        whitelist.add(user2);\\n        vm.stopPrank();\\n\\n        vm.startPrank(admin);\\n\\n        // add depositors\\n        vault.add_depositor(admin);\\n        vault.add_depositor(user1);\\n        vault.add_depositor(user2);\\n\\n        // check yield token is whitelisted\\n        assert(alchemist.isSupportedYieldToken(yieldToken));\\n\\n        // mint NFTs to various parties\\n        nft.mint(admin, 1);\\n        nft.mint(user1, 2);\\n        nft.mint(user2, 3);\\n        \\n\\n        // give max WETH approval to vault & alchemist\\n        weth.approve(vaultAdd, type(uint256).max);\\n        weth.approve(address(alchemist), type(uint256).max);\\n\\n        // send some WETH to user1 & user2\\n        weth.transfer(user1, 10e18);\\n        weth.transfer(user2, 10e18);\\n\\n        // users give WETH approval to vault and alchemist\\n        vm.stopPrank();\\n        vm.startPrank(user1);\\n        weth.approve(vaultAdd, type(uint256).max);\\n        weth.approve(address(alchemist), type(uint256).max);\\n        vm.stopPrank();\\n        vm.startPrank(user2);\\n        weth.approve(vaultAdd, type(uint256).max);\\n        weth.approve(address(alchemist), type(uint256).max);\\n        vm.stopPrank();\\n\\n        // by default, msg.sender will be admin\\n        vm.startPrank(admin);\\n    }\\n\\n    function testVaultLiquidationAfterRepayment() public {\\n        uint256 depositAmt = 1e18;\\n        // admin does a deposit\\n        vault.register_deposit(1, depositAmt);\\n        vm.stopPrank();\\n\\n        // user1 does a deposit too\\n        vm.prank(user1);\\n        vault.register_deposit(2, depositAmt);\\n\\n        // simulate yield: someone does partial manual repayment\\n        vm.prank(user2);\\n        alchemist.repay(address(weth), 0.1e18, vaultAdd);\\n\\n        // mark it as claimable (burn a little bit more shares because of rounding)\\n        vault.withdraw_underlying_to_claim(\\n            alchemist.convertUnderlyingTokensToShares(yieldToken, 0.01e18) + 100,\\n            0.01e18\\n        );\\n\\n        vm.stopPrank();\\n\\n        // user1 performs liquidation, it's fine\\n        vm.prank(user1);\\n        vault.liquidate(2, 0);\\n\\n        // assert that admin has more shares than what the vault holds\\n        (uint256 shares, ) = alchemist.positions(vaultAdd, yieldToken);\\n        IVault.Position memory adminPosition = vault.positions(1);\\n        assertGt(adminPosition.sharesOwned, shares);\\n\\n        vm.prank(admin);\\n        // now admin is unable to liquidate because of contract doesn't hold sufficient shares\\n        // expect Arithmetic over/underflow error\\n        vm.expectRevert(stdError.arithmeticError);\\n        vault.liquidate(1, 0);\\n    }\\n}\\n```\\n
For the `shares_to_liquidate` and `amount_to_withdraw` variables, check against the vault's current shares and take the minimum of the 2.\\nThe better fix would be to switch from marking yield claims with withdrawing WETH collateral to minting debt (alETH) tokens.
Failing liquidations as the contract attempts to burn more shares than it holds.
```\\n// SPDX-License-Identifier: MIT\\npragma solidity 0.8.18;\\n\\nimport "forge-std/Test.sol";\\nimport "../../lib/utils/VyperDeployer.sol";\\n\\nimport "../IVault.sol";\\nimport "../IAlchemistV2.sol";\\nimport "../MintableERC721.sol";\\nimport "openzeppelin/token/ERC20/IERC20.sol";\\n\\ncontract VaultTest is Test {\\n    ///@notice create a new instance of VyperDeployer\\n    VyperDeployer vyperDeployer = new VyperDeployer();\\n\\n    FairFundingToken nft;\\n    IVault vault;\\n    address vaultAdd;\\n    IAlchemistV2 alchemist = IAlchemistV2(0x062Bf725dC4cDF947aa79Ca2aaCCD4F385b13b5c);\\n    IWhitelist whitelist = IWhitelist(0xA3dfCcbad1333DC69997Da28C961FF8B2879e653);\\n    address yieldToken = 0xa258C4606Ca8206D8aA700cE2143D7db854D168c;\\n    IERC20 weth = IERC20(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2);\\n    // pranking from big WETH holder\\n    address admin = 0x2fEb1512183545f48f6b9C5b4EbfCaF49CfCa6F3;\\n    address user1 = address(0x123);\\n    address user2 = address(0x456);\\n    \\n    function setUp() public {\\n        vm.startPrank(admin);\\n        nft = new FairFundingToken();\\n        /// @notice: I modified vault to take in admin as a parameter\\n        /// because of pranking issues => setting permissions\\n        vault = IVault(\\n            vyperDeployer.deployContract("Vault", abi.encode(address(nft), admin))\\n        );\\n        // to avoid having to repeatedly cast to address\\n        vaultAdd = address(vault);\\n        vault.set_alchemist(address(alchemist));\\n\\n        // whitelist vault and users in Alchemist system, otherwise will run into permission issues\\n        vm.stopPrank();\\n        vm.startPrank(0x9e2b6378ee8ad2A4A95Fe481d63CAba8FB0EBBF9);\\n        whitelist.add(vaultAdd);\\n        whitelist.add(admin);\\n        whitelist.add(user1);\\n        whitelist.add(user2);\\n        vm.stopPrank();\\n\\n        vm.startPrank(admin);\\n\\n        // add depositors\\n        vault.add_depositor(admin);\\n        vault.add_depositor(user1);\\n        vault.add_depositor(user2);\\n\\n        // check yield token is whitelisted\\n        assert(alchemist.isSupportedYieldToken(yieldToken));\\n\\n        // mint NFTs to various parties\\n        nft.mint(admin, 1);\\n        nft.mint(user1, 2);\\n        nft.mint(user2, 3);\\n        \\n\\n        // give max WETH approval to vault & alchemist\\n        weth.approve(vaultAdd, type(uint256).max);\\n        weth.approve(address(alchemist), type(uint256).max);\\n\\n        // send some WETH to user1 & user2\\n        weth.transfer(user1, 10e18);\\n        weth.transfer(user2, 10e18);\\n\\n        // users give WETH approval to vault and alchemist\\n        vm.stopPrank();\\n        vm.startPrank(user1);\\n        weth.approve(vaultAdd, type(uint256).max);\\n        weth.approve(address(alchemist), type(uint256).max);\\n        vm.stopPrank();\\n        vm.startPrank(user2);\\n        weth.approve(vaultAdd, type(uint256).max);\\n        weth.approve(address(alchemist), type(uint256).max);\\n        vm.stopPrank();\\n\\n        // by default, msg.sender will be admin\\n        vm.startPrank(admin);\\n    }\\n\\n    function testVaultLiquidationAfterRepayment() public {\\n        uint256 depositAmt = 1e18;\\n        // admin does a deposit\\n        vault.register_deposit(1, depositAmt);\\n        vm.stopPrank();\\n\\n        // user1 does a deposit too\\n        vm.prank(user1);\\n        vault.register_deposit(2, depositAmt);\\n\\n        // simulate yield: someone does partial manual repayment\\n        vm.prank(user2);\\n        alchemist.repay(address(weth), 0.1e18, vaultAdd);\\n\\n        // mark it as claimable (burn a little bit more shares because of rounding)\\n        vault.withdraw_underlying_to_claim(\\n            alchemist.convertUnderlyingTokensToShares(yieldToken, 0.01e18) + 100,\\n            0.01e18\\n        );\\n\\n        vm.stopPrank();\\n\\n        // user1 performs liquidation, it's fine\\n        vm.prank(user1);\\n        vault.liquidate(2, 0);\\n\\n        // assert that admin has more shares than what the vault holds\\n        (uint256 shares, ) = alchemist.positions(vaultAdd, yieldToken);\\n        IVault.Position memory adminPosition = vault.positions(1);\\n        assertGt(adminPosition.sharesOwned, shares);\\n\\n        vm.prank(admin);\\n        // now admin is unable to liquidate because of contract doesn't hold sufficient shares\\n        // expect Arithmetic over/underflow error\\n        vm.expectRevert(stdError.arithmeticError);\\n        vault.liquidate(1, 0);\\n    }\\n}\\n```\\n