Epoch Island security record

When calling `withdrawForfeit()`, the rewards that need to be paid back to the contract is calculated as follows: ``` uint256 forfeitReward = ((earned[_depositId] + rewardTokensClaimed[_depositId]) * percentage) / 1e18; ``` However, the `rewardTokensClaimed` is never updated after the payback. Thus, when users don't withdraw 100 percent of their deposit, they will repay more rewards than they are obligated to with the following withdrawals. Furthermore, the user won't be able to unlock the entire deposit unless they buy `rewardToken` to pay back.

The taker has no economic incentive to act upon desired positions immediately. The reason is that the protocol and LP decrease towards the position end. As a result, the conditions for the positions become better for the taker over time. If a taker acts ideally in terms of game theory, they would wait with the trade as long as possible. Therefore, their ideal strategy would be to observe the mempool until they see a transaction that would prevent their transaction and front-run it. A transaction prevents the takers transaction if it reduces the available `downsideToken` amount below the desired amount.

The following functions are violating the "Checks-Effects-Interactions"-pattern and/or do not employ any reentrancy mitigating modifers, making them susceptible to reentrancy: 1. `EpochUpsidePoolV1.supply()`. 1. `EpochUpsidePoolV1.remove()`. 1. `EpochUpsidePoolV1.take()`. 1. `EpochUpsidePoolV1.untake()`. 1. `EpochUpsidePoolV1.claimProtocolFees()`. 1. `EpochUpsidePoolV1.claimPositionFees()`.

An attacker may be able to reduce the rewards received by other users without any penalty, only incurring gas costs. Such an attack is possible whenever `forfeitRedirectionAddress != address(0)`. The described exploit scenario will lead to the fact that current deposits will receive fewer reward shares. In case the attacker has control over `forfeitRedirectionAddress`, they will be able to claim those rewards without any deposits being locked.

In the revised `take()` function, there is an issue concerning the calculation of token amounts when the provided `_maxDownsideTokenIn` is larger than the available liquidity. This situation triggers a reverse calculation to adjust the amount of downside tokens after fees (`_downsideTokenBalanceAfterFees`). However, the current implementation may lead to a scenario where the sum of `_downsideTokenBalanceAfterFees`, `protocolFee`, and `lpFee` does not equal the initial `_maxDownsideTokenIn`. This discrepancy can result in users unintentionally committing a higher amount of downside tokens than intended, especially when liquidity is low, leading to unexpected financial implications.

We could identify an incident of inefficient gas usage: `lpPositions` is a mapping, although it is treated like an array. It is more expensive to store new values in a mapping as opposed to an array.

Every Solidity file specifies in the header a version number of the format `pragma solidity (^)0.8.*`. The caret (`^`) before the version number implies an unlocked pragma, meaning that the compiler will use the specified version _and above_, hence the term "unlocked".

The following lines are using the 'unsafe' ERC20 interface functions (`transfer()`): 1. `EpochUpsidePoolV1.sol#L87`. 1. `EpochUpsidePoolV1.sol#L91`. 1. `EpochUpsidePoolV1.sol#L136`. 1. `EpochUpsidePoolV1.sol#L156`. 1. `EpochUpsidePoolV1.sol#L180`. 1. `EpochUpsidePoolV1.sol#L191`. Further, their return values were not checked, which, depending on token, could lead to failed transactions but continued code execution, incorrect accounting within the `EpochUpsidePoolV1.sol` contract and ultimately to loss of funds for the protocol/participants. Since [some ERC20 tokens do not return a value on above mentioned functions/behave non-uniformly](https://medium.com/coinmonks/missing-return-value-bug-at-least-130-tokens-affected-d67bf08521ca) it is advised to use a secure wrapping interface around said functions, like for example OpenZeppelins [SafeERC20](https://docs.openzeppelin.com/contracts/2.x/api/token/erc20#SafeERC20), which implicitly checks for the different error cases.

The [Checks-Effects-Interactions](https://solidity.readthedocs.io/en/latest/security-considerations.html#use-the-checks-effects-interactions-pattern) coding pattern is meant to mitigate any chance of other contracts manipulating the state of the blockchain in unexpected and possibly malicious ways before control is returned to the original contract. As the name implied, only after checking whether appropriate conditions are met and acting internally on those conditions should any external calls to, or interactions with, other contracts be done. The following functions are violating the "Checks-Effects-Interactions"-pattern and/or do not employ any reentrancy mitigating modifers, making them susceptible to reentrancy: 1. `Vepoch.addRewardTokens()`. 1. `Vepoch.claimYield()`. 1. `Vepoch.deposit()`. 1. `Vepoch.withdrawForfeit()`. 1. `Vepoch.withdraw()`.

Every Solidity file specifies in the header a version number of the format `pragma solidity (^)0.8.*`. The caret (`^`) before the version number implies an unlocked pragma, meaning that the compiler will use the specified version _and above_, hence the term "unlocked".