Illuvium Yield Farming Rewards

The function `swapEthForIlv` uses Uniswap to exchange ETH for ILV tokens. The function is declared public with no access control. If the contract holds a large amount of ETH, attackers can manipulate (likely using flash loans) the Uniswap ETH/ILV price such that the `IlluviumVault` will receive an unfavorable amount of ILV. Further, computing `ilvOut` in the function leaves it susceptible to [sandwich attacks](https://medium.com/coinmonks/demystify-the-dark-forest-on-ethereum-sandwich-attacks-5a3aec9fa33e).

The `_processVaultRewards` function inside `IlluviumCorePool` and in `IlluviumLockedPool` will not give users the full amount of the reward they are entitled to, when `pendingVaultClaim > poolTokenReserve`. Moreover, the function will also stop the users from requesting for the missing amount afterward. Hence the users will lose rewards.

The `IlluviumPoolBase._updateStakeLock` function does not flush rewards before changing the value of `user.totalWeight`. This may lead to incorrect reward amounts subsequently.

If the ETH balance of `IlluviumVault` is zero (possibly due to a previous call to either `swapEthForIlv` or `sendIlvRewards`), the function will revert due to the check `balance > 0` on L159. However, since `swapEthForIlv` is public, a legitimate call to `sendIlvRewards` could be griefed by any user if they front-run with a call to `swapEthForIlv`.

According to the inline documentation, step 2 should invoke `setPool`, and step 3 sets balances (through potentially multiple calls to `setBalances`). However, the function `setBalances` requires on L164 that `address(pool) == address(0)`, so step 3 cannot occur after step 2. The steps on L17-20 appear correct, but L126 and L149 do not align with this summary.

In the `IlluviumCorePool.stakeAsPool` function on L155 code comment says that the `_useSILV` should be `false`, however it's not in the case on L165 where the 2nd parameter passed to the `_processRewards` function, which represents the value of `_useSILV` is hardcoded to `true`.

Assumptions about intermediate values during function processing should be explicitly checked, especially if these values depend on outputs returned by external contract calls. For example, we assume that the value of `stakeWeight` on L420 inside of `IlluviumPoolBase._stake()` should be greater than zero. Otherwise, it doesn't make sense to create a deposit with `stakeWeight == 0`. <br><br> Note that this is one example of what we assume to be an implicit assumption, however, all implicit assumptions should be checked in a similar way.

The following instances of missing or insufficient input validation have been encountered: 1. The `_pool` parameter of the of the `TokenLocking.setPool()` function is not checked to conform to the `ILockedPool` interface and could be any address. 2. The `_rewardsAmount` parameter of the `IlluviumLockedPool.receiveVaultRewards()` function is not checked to be greater than zero. The same applies to the function with the same name in other contracts. 3. The `_from` and `_to` parameters of `IlluviumLockedPool.changeLockedHolder()` are not checked to be different. This could lead to deleting a holder. 4. The `_vault` parameter of `IlluviumCorePool.setVault()` is not checked to be different from `address(0)`. 5. The `_weight` parameter of `IlluviumPoolBase.setWeight()` is not checked to be greater than zero. Note that this is not an exhaustive list. User inputs should always be validated.

The `TokenLocking.setBalances()` function may be called multiple times in order to "allow setting balance to zero in case of accidental addition of the holder". However, due to missing checks/assertions there exists a possibility for human error which could lead to unlocking a total balance larger than intended.

In `IlluviumCorePool.sol` on L30, the comment states that `poolTokenReserve` is the "Total value of ILV tokens available in the pool". However, while functions such as `IlluviumCorePool.receiveVaultRewards` only increase `poolTokenReserve` when `poolToken == ilv`, this is not the case for functions such as `IlluviumCorePool._stake`. For example, if the `poolToken` is `ILV/ETH Pair`, the `poolTokenReserve` is still increased on L205, even though pair tokens are staked (not ILV). This makes the computation in `IlluviumVault.sendIlvRewards` of `ilvInPairPool` unclear: ``` uint256 ilvInPairPool = (pairPoolReserve.mul(ilv.balanceOf(address(ilvEthPair))).div(ilvEthPair.totalSupply())).add( pairPoolIlvBalance ); ``` In particular, the expression `(pairPoolReserve.mul(ilv.balanceOf(address(ilvEthPair))).div(ilvEthPair.totalSupply()))` seems to suggest that `pairPoolReserve` should store the amount of `ILV/ETH Pair` tokens in the `ILV/ETH Pair` pool, *NOT* the ILV balance itself. With the current setup, it appears that the `ilvInPairPool` computation above will double-count some tokens, since `pairPoolReserve` is increased for both `ILV` and `ILV/ETH Pair` deposits. This will inflate the weight associated with the `ILV/ETH Pair` pool.