FEY

The current method of creating token approvals is vulnerable to [a known ERC20 race condition issue](https://github.com/ethereum/EIPs/issues/20#issuecomment-263524729), which could lead to potential loss of tokens. When a user calls `approve()` for a second time to a spender that has already been approved, there is a possibility that the spender could call `transferFrom()` to transfer the amount from the original approval. The spender will then receive the approval to transfer the new value. Instead of having the new approval replace the original approval value, this transaction flow has allowed the spender to spend the tokens from both approvals.

If an event is not indexed in a smart contract, it means that the event's parameters are not tagged with the `indexed` keyword. This has implications for how the event data can be searched and filtered when looking through blockchain logs. Without indexing, the event will still emit the data as part of the transaction log, but users won't be able to query for these events using the parameters. They'll have to retrieve the entire set of logs and manually sift through them to find events with the specific data. This can be less efficient and more time-consuming, especially on a blockchain with a high volume of transactions and events.

The function `getStakeAge()` returns `MAX_STAKE_DAYS` if the total time staked exceeds `MAX_STAKE_DAYS`. Then if the total time staked exceeds `MAX_STAKE_DAYS`, then the function `getStakeInterest()` calculates the interest of the stake from the current day minus `MAX_STAKE_DAYS` until the current day. Thus if a user staked for an extra day, their interest would be calculated from the second day on. A user may stake for longer if the initial days of their stake have a lower interest rate than the interest rate of the most recent days. Note that this can be handled if `closeGhostStake()` is ensured to be called after a users stake reaches the `MAX_STAKE_DAYS`, so we give this an *informational* severity.

In the function `getInterest()`, there is the following calculations ```solidity uint256 dailyInterestAmt = getPercent( yearFullAmount, 31556952, 0 ); durationInterestAmt = dailyInterestAmt .mul(_seconds) .div(100); ``` It appears the design intent is to divide the `yearFullAmount` by the amount of seconds per year to the determine the amount of interest per second that should be allocated. However, the name `dailyInterestAmt` does not seem to be consistent with this value. Furthermore, it is divided by `31556952`, when the number of seconds in a non leap year is `31536000` and the number of seconds in a solar year is `31556736`.

### FeyorraBridge - In the function `transferFeyorraRequest()` for non-custom chains and custom chains the input `_amount` is not checked to be greater than 0. If 0 is input, then it may bridge 0 tokens unnecessarily. - In the function `transferFeyorraRequest()` for non-custom chains the input `_gasLimit` is not checked to be enough to successfully call `ccipReceive()`. If this amount is not large enough, then the bridging will have to be manually executed later. See [https://docs.chain.link/ccip/tutorials/ccipreceive-gaslimit#overview](https://docs.chain.link/ccip/tutorials/ccipreceive-gaslimit#overview). - In the function `manualExecuteBridgeTransfer()` the source chain and sender are not checked. This can result in a non-supported source chain or sender. ### Token - In the function `multiTransfer()` it is not checked that the input `_destinations` and `_values` have the same length. This can lead to improper transfers or a revert.

The cited inputs are missing a check that it is not `address(0)` or in case it is an address on another chain that it is not the null address on that chain.

Chainlink's best practices recommend decoupling message reception and business logic: [https://docs.chain.link/ccip/best-practices#decoupling-ccip-message-reception-and-business-logic](https://docs.chain.link/ccip/best-practices#decoupling-ccip-message-reception-and-business-logic). However, the contract does not have a method to store or retry failed messages. While these can then be attempted via Manual Execution, the revert may be caused by an issue that cannot be resolved. For example, in the current code, if `transferFeyorraRequest()` is called with the input `_receiver = address(0)` then the request will be successful. However, any attempt to execute the bridge transfer on the destination chain will fail as transfers of `FEY` revert if the receiver is the zero address. This may result in issues with the protocol as the corresponding `requestId` will have emitted a `TransferFeyorraRequest` event, but no `ExecuteBridgeTransfer` event will ever be emitted.

The function `manualExecuteBridgeTransfer()` checks that the input `_sourceChainSelector` is a custom chain otherwise it will revert. If a chain is updated to be a custom chain after a request has been sent, but before it is executed, then this will prevent the bridging process from being completed.

The linked statements do not affect the functionality of the codebase and appear to be either leftovers from test code or older functionality.

Since Solidity Release 0.8.23, `MCOPY` opcode is introduced. However, this may not be compatible with all chains and L2s. As a result, the compatibility of the code could be affected.